fix(onboard): replace predictable temp filenames with mkdtempSync#1094
Conversation
Probe functions and writeSandboxConfigSyncFile use Date.now() and Math.random() to construct temp filenames in os.tmpdir(). These are predictable, allowing a local attacker to race the file creation with a symlink and redirect curl output (which may contain API responses) to an attacker-controlled path. Replace all 6 sites with fs.mkdtempSync() which creates a directory with a cryptographically random suffix and restrictive permissions. This matches the pattern already used at lines 1764 and 2680 in the same file. Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughReplaced predictable timestamp/random temp filenames with per-invocation unique directories created via Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…test Accept both forward and back slashes in the path regex and skip the Unix file permission assertion on Windows where mode bits are not enforced. Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
…m/latenighthackathon/NemoClaw into fix/onboard-predictable-tempfiles
…IDIA#1094) * fix(onboard): replace predictable temp filenames with mkdtempSync Probe functions and writeSandboxConfigSyncFile use Date.now() and Math.random() to construct temp filenames in os.tmpdir(). These are predictable, allowing a local attacker to race the file creation with a symlink and redirect curl output (which may contain API responses) to an attacker-controlled path. Replace all 6 sites with fs.mkdtempSync() which creates a directory with a cryptographically random suffix and restrictive permissions. This matches the pattern already used at lines 1764 and 2680 in the same file. Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com> * fix(test): handle cross-platform paths in writeSandboxConfigSyncFile test Accept both forward and back slashes in the path regex and skip the Unix file permission assertion on Windows where mode bits are not enforced. Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com> --------- Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com> Co-authored-by: latenighthackathon <latenighthackathon@users.noreply.github.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Summary
Probe functions and
writeSandboxConfigSyncFileconstruct temp filenames usingDate.now()andMath.random(), both of which are predictable. A local attacker can race the file creation with a symlink to redirect curl output (API responses) or inject a malicious sync script. This replaces all 6 sites withfs.mkdtempSync(), matching the secure pattern already used elsewhere in the same file.Related Issue
Closes #1093
Changes
fs.mkdtempSync():writeSandboxConfigSyncFile— now createsnemoclaw-sync-<random>/sync.shprobeOpenAiLikeEndpoint— now createsnemoclaw-probe-<random>/body.jsonprobeAnthropicEndpoint— now createsnemoclaw-anthropic-probe-<random>/body.jsonfetchNvidiaEndpointModels— now createsnemoclaw-nvidia-models-<random>/body.jsonfetchOpenAiLikeModels— now createsnemoclaw-openai-models-<random>/body.jsonfetchAnthropicModels— now createsnemoclaw-anthropic-models-<random>/body.jsonfinallyblocks to clean up the temp directory ({ recursive: true }) instead of just the file.setupOpenclawto remove the temp directory instead of unlinking the file.Testing
npm testpasses (no local environment)Executed:
mkdtempSynccalls use the correct prefix to preserve debuggabilityfinallyblocks referenceprobeDir(notbodyFile) for recursive cleanupwriteSandboxConfigSyncFilecaller at line 2373 cleans up viapath.dirname(scriptFile)fs.mkdtempSyncis already used at lines 1764 and 2680 in the same file — no new API dependencyChecklist
Summary by CodeRabbit
Chores
Tests